You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Removed the parameterless constructor from RelativeBy, because it does not provide a value for root and this value is crucial to RelativeBy's functionality.
Motivation and Context
Fixes null warning on RelativeBy.root and prevents extending the RelativeBy type.
Currently, if you try to extend the type, you get an error:
System.ArgumentNullException : object to serialize must not be null (Parameter 'root')
RelativeBy.GetSerializableObject(Object root) line 379
RelativeBy.FindElements(ISearchContext context) line 106
WebDriver.FindElements(By by) line 377
Types of changes
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
The new code initializes filters with an empty list literal [] which is a C# 12 feature. Ensure this is compatible with the project's target framework and compiler version.
✅ Add missing sealed modifierSuggestion Impact:The commit directly implemented the suggestion by adding the 'sealed' modifier to the RelativeBy class, which was appropriate since the implementation prevents inheritance.
code diff:
- public class RelativeBy : By+ public sealed class RelativeBy : By
The class should be explicitly marked as sealed since the implementation now prevents inheritance by removing the protected constructor and making all constructors private.
-public class RelativeBy : By+public sealed class RelativeBy : By
{
private readonly string wrappedAtom;
private readonly object root;
private readonly List<object> filters;
[Suggestion has been applied]
Suggestion importance[1-10]: 7
__
Why: The suggestion correctly identifies that the class should be marked as sealed since the implementation prevents inheritance by removing the protected constructor and making all constructors private. This improves code clarity and prevents potential misuse.
Medium
Learned best practice
✅ Use explicit collection initialization with new List<object>() instead of collection expressions for better readability and consistency with .NET conventionsSuggestion Impact:The commit addressed the same issue as the suggestion by replacing the collection expression syntax with explicit initialization. Instead of using 'new List()' as suggested, the implementation used field initialization with 'new List()' and then 'AddRange' for populating the list when filters are not null.
The current implementation uses collection expressions [] to initialize an empty list when filters is null. While this works, it's better to use new List() or the collection initializer syntax new() for consistency with .NET conventions and to make the code more explicit about the type being created.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Description
Removed the parameterless constructor from
RelativeBy, because it does not provide a value forrootand this value is crucial toRelativeBy's functionality.Motivation and Context
Fixes null warning on
RelativeBy.rootand prevents extending theRelativeBytype.Currently, if you try to extend the type, you get an error:
Types of changes
Checklist
PR Type
Bug fix, Enhancement
Description
Sealed the
RelativeByclass to prevent extension.Removed the parameterless constructor from
RelativeBy.Refactored
filtersinitialization to handle null values.Moved
wrappedAtominitialization to a static method.Changes walkthrough 📝
RelativeBy.cs
Refactor and seal `RelativeBy` classdotnet/src/webdriver/RelativeBy.cs
RelativeByclass to prevent extension.filtersinitialization to handle null values.wrappedAtominitialization to a static helper method.